ref: Use CryptoKit instead of CommonCrypto in SentryDsn#7037
ref: Use CryptoKit instead of CommonCrypto in SentryDsn#7037
Conversation
antonis
requested review from
itaybre,
noahsmartin,
philipphofmann and
philprime
as code owners
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
|
||
| ### Breaking Changes | ||
|
|
||
| - Bumped minimum macOS version from 10.14.0 to 10.15.0 |
There was a problem hiding this comment.
Thank you for the feedback. Given that the CryptoKit API is not available I suggest to close the PR. I've also added the Blocked label
There was a problem hiding this comment.
Can't we use it for newer macOS versions?
There was a problem hiding this comment.
I guess we can conditionally use CryptoKit and fallback to CommonCrypto when not available.
I think this would make things more complicated though and beat the purpose of using a simpler Swift friendly syntax.
There was a problem hiding this comment.
If there is not clear advantage to conditionally use CryptoKit let's not do this change.
There was a problem hiding this comment.
Sounds good 👍
I'm closing this PR. We can revisit it when bumping macos is possible.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7037 +/- ##
=============================================
- Coverage 85.027% 84.690% -0.337%
=============================================
Files 454 449 -5
Lines 27737 27415 -322
Branches 12159 12003 -156
=============================================
- Hits 23584 23218 -366
- Misses 4107 4150 +43
- Partials 46 47 +1
... and 71 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
2 participants
📜 Description
CryptoKitinstead ofCommonCryptoinSentryDsn10.14to10.15that theInsecure.SHA1from CryptoKit requires💡 Motivation and Context
See #6942 (comment)
This is a swift improvement as a follow up to the above review feedback. It is not required thus feel free to reject/close the PR if the (breaking) change is not desired.
💚 How did you test it?
📝 Checklist
You have to check all boxes before merging:
sendDefaultPIIis enabled.#skip-changelog
Closes #7038